Include the updated header from Geometry/Records instead of the obsolete one #33784

perrotta · 2021-05-19T16:37:09Z

PR description:

The header file Geometry/CaloEventSetup/interface/CaloTopologyRecord.h was since ages simply pointing to the correct Geometry/Records/interface/CaloTopologyRecord.h, as it reads

#ifndef Geometry_CaloEventSetyp_CaloTopologyRecord
#define Geometry_CaloEventSetyp_CaloTopologyRecord

//moved so just forward the new one under the old name

#include "Geometry/Records/interface/CaloTopologyRecord.h"

#endif

This PR replaces all remaining occurrencies of "Geometry/CaloEventSetup/interface/CaloTopologyRecord.h" with "Geometry/Records/interface/CaloTopologyRecord.h" in CMSSW, and remove the obsolete header.

As such it is purely technical, and does nothing but simplifying the dependencies and cleaning the release from obsolete material: if @cms-sw/geometry-l2 agrees with the move, I think this can be safely merged.

PS: I hoped I could have avoided applyng the code-format to the code RecoJets/JetAnalyzers/test/myJetAna.cc in the test area, but unfortunately it was not the case... With the second commit of this PR we add thousands of useless code format related updates to that test code. At least, they do not look harmful.

PR validation:

It builds, and the short matrix runs

cmsbuild · 2021-05-19T16:47:05Z

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-33784/22751

This PR adds an extra 640KB to repository
There are other open Pull requests which might conflict with changes you have proposed:
- File DQMOffline/CalibCalo/src/DQMSourcePi0.h modified in PR(s): [RFC] Test clang-tidy --checks misc-definitions-in-headers #33366
- File RecoEgamma/EgammaPhotonProducers/src/GEDPhotonProducer.cc modified in PR(s): Calotower -> HCAL rechit for EGM ID variables #33666
- File RecoEgamma/EgammaPhotonProducers/src/PhotonProducer.cc modified in PR(s): Calotower -> HCAL rechit for EGM ID variables #33666

Code check has found code style and quality issues which could be resolved by applying following patch(s)

code-format:
https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-33784/22751/code-format.patch
e.g. curl https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-33784/22751/code-format.patch | patch -p1
You can also run scram build code-format to apply code format directly

cmsbuild · 2021-05-19T17:04:28Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-33784/22752

This PR adds an extra 656KB to repository
There are other open Pull requests which might conflict with changes you have proposed:
- File DQMOffline/CalibCalo/src/DQMSourcePi0.h modified in PR(s): [RFC] Test clang-tidy --checks misc-definitions-in-headers #33366
- File RecoEgamma/EgammaPhotonProducers/src/GEDPhotonProducer.cc modified in PR(s): Calotower -> HCAL rechit for EGM ID variables #33666
- File RecoEgamma/EgammaPhotonProducers/src/PhotonProducer.cc modified in PR(s): Calotower -> HCAL rechit for EGM ID variables #33666

cmsbuild · 2021-05-19T17:04:34Z

A new Pull Request was created by @perrotta for master.

It involves the following packages:

Calibration/EcalAlCaRecoProducers
Calibration/EcalCalibAlgos
Calibration/HcalAlCaRecoProducers
Calibration/HcalCalibAlgos
Calibration/IsolatedParticles
Calibration/Tools
Configuration/Skimming
DPGAnalysis/Skims
DQMOffline/CalibCalo
DQMOffline/EGamma
DQMOffline/Ecal
DQMOffline/Trigger
EgammaAnalysis/ElectronTools
FastSimulation/CaloGeometryTools
FastSimulation/CaloHitMakers
FastSimulation/EventProducer
FastSimulation/SimplifiedGeometryPropagator
Geometry/CaloEventSetup
HLTrigger/Egamma
HLTrigger/special
PhysicsTools/PatAlgos
RecoCaloTools/Navigation
RecoEcal/EgammaClusterAlgos
RecoEcal/EgammaClusterProducers
RecoEcal/EgammaCoreTools
RecoEgamma/EgammaHLTProducers
RecoEgamma/EgammaPhotonAlgos
RecoEgamma/EgammaPhotonProducers
RecoEgamma/EgammaTools
RecoEgamma/ElectronIdentification
RecoEgamma/Examples
RecoEgamma/PhotonIdentification
RecoHI/HiEgammaAlgos
RecoJets/JetAnalyzers
RecoLocalCalo/EcalRecProducers
RecoMET/METFilters
RecoMuon/MuonIdentification
Validation/EcalClusters
Validation/RecoEgamma

@andrius-k, @lveldere, @chayanit, @wajidalikhan, @sbein, @ianna, @Martin-Grunewald, @tlampen, @pohsun, @santocch, @perrotta, @civanch, @yuanchao, @makortel, @ErnestaP, @ahmad3213, @cmsbuild, @fwyzard, @Dr15Jones, @cvuosalo, @mdhildreth, @jfernan2, @slava77, @jpata, @francescobrivio, @malbouis, @ssekmen, @jordan-martins, @kmaeshima, @rvenditti can you please review it and eventually sign? Thanks.
@emilbols, @gouskos, @jainshilpi, @hatakeyamak, @rappoccio, @apsallid, @argiro, @Martin-Grunewald, @Fedespring, @thomreis, @mbluj, @missirol, @lgray, @abbiendi, @varuns23, @seemasharmafnal, @mmarionncern, @JyothsnaKomaragiri, @ahinzmann, @jhgoh, @dgulhan, @jdolen, @sobhatta, @cericeci, @simonepigazzini, @ferencek, @trocino, @yetkinyilmaz, @sscruz, @Sam-Harper, @afiqaize, @rovere, @jdamgov, @bellan, @nhanvtran, @gkasieczka, @tocheng, @schoef, @mmusich, @mtosi, @fabiocos, @clelange, @HuguesBrun, @rchatter, @mandrenguyen, @swozniewski, @jazzitup, @calderona, @yenjie, @lecriste, @kurtejung, @gpetruc, @matt-komm, @mariadalfonso, @rociovilar, @andrzejnovak, @ram1123 this is something you requested to watch as well.
@silviodonato, @dpiparo, @qliphy you are the release manager for this.

cms-bot commands are listed here

perrotta · 2021-05-19T17:05:37Z

please test

cmsbuild · 2021-05-19T17:45:18Z

-1

Failed Tests: Build
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-4a008b/15189/summary.html
COMMIT: 41a3eda
CMSSW: CMSSW_12_0_X_2021-05-18-2300/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/33784/15189/install.sh to create a dev area with all the needed externals and cmssw changes.

CMS Clang-Tidy warnings: There are Clang-Tidy warnings. See https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-4a008b/15189/llvm-analysis/cmsclangtidy.txt for details.

Build

I found compilation error when building:

>> Compiling edm plugin /build/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_12_0_X_2021-05-18-2300/src/CaloOnlineTools/EcalTools/plugins/EcalFEDErrorFilter.cc
>> Compiling edm plugin /build/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_12_0_X_2021-05-18-2300/src/CaloOnlineTools/EcalTools/plugins/EcalFEDWithCRCErrorProducer.cc
>> Compiling edm plugin /build/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_12_0_X_2021-05-18-2300/src/CaloOnlineTools/EcalTools/plugins/EcalHexDisplay.cc
>> Compiling edm plugin /build/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_12_0_X_2021-05-18-2300/src/CaloOnlineTools/EcalTools/plugins/EcalMipGraphs.cc
In file included from /build/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_12_0_X_2021-05-18-2300/src/CaloOnlineTools/EcalTools/plugins/EcalCosmicsHists.cc:17:
/build/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_12_0_X_2021-05-18-2300/poison/Geometry/CaloEventSetup/interface/CaloTopologyRecord.h:1:2: error: #error THIS FILE HAS BEEN REMOVED FROM THE PACKAGE.
    1 | #error THIS FILE HAS BEEN REMOVED FROM THE PACKAGE.
      |  ^~~~~
In file included from /build/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_12_0_X_2021-05-18-2300/src/CaloOnlineTools/EcalTools/plugins/EcalDisplaysByEvent.h:46,
                 from /build/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_12_0_X_2021-05-18-2300/src/CaloOnlineTools/EcalTools/plugins/EcalDisplaysByEvent.cc:19:
/build/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_12_0_X_2021-05-18-2300/poison/Geometry/CaloEventSetup/interface/CaloTopologyRecord.h:1:2: error: #error THIS FILE HAS BEEN REMOVED FROM THE PACKAGE.

chayanit · 2021-05-20T08:42:03Z

+1

cvuosalo · 2021-05-20T16:13:55Z

+1

perrotta · 2021-05-20T16:24:55Z

+1

Technical
Jenkins tests pass

perrotta · 2021-05-24T13:58:31Z

@cms-sw/alca-l2 @cms-sw/analysis-l2 @cms-sw/fastsim-l2 : could you please have a look and sign, if you agree with it?

civanch · 2021-05-24T14:44:59Z

+1

yuanchao · 2021-05-25T09:06:17Z

+1

replace and remove the obsolete header (technical)
matrix tests and unit tests passed

qliphy · 2021-05-25T09:27:48Z

+1

qliphy · 2021-05-25T09:28:22Z

merge

santocch · 2021-05-30T10:16:43Z

+1

cmsbuild · 2021-05-30T10:17:10Z

This pull request is fully signed and it will be integrated in one of the next master IBs (tests are also fine). This pull request will be automatically merged.

Include the updated header file instead of the obsolete one

5c593f9

cmsbuild added this to the CMSSW_12_0_X milestone May 19, 2021

cmsbuild added alca-pending analysis-pending code-checks-pending dqm-pending fastsim-pending geometry-pending hlt-pending orp-pending pdmv-pending pending-signatures reconstruction-pending tests-pending labels May 19, 2021

perrotta changed the title ~~Include the updated header file instead of the obsolete one~~ Include the updated header from Geometry/Records instead of the obsolete one May 19, 2021

cmsbuild added code-checks-rejected and removed code-checks-pending labels May 19, 2021

apply code-format

41a3eda

cmsbuild added code-checks-pending and removed code-checks-rejected labels May 19, 2021

cmsbuild added code-checks-approved and removed code-checks-pending labels May 19, 2021

cmsbuild added tests-started and removed tests-pending labels May 19, 2021

cmsbuild removed the tests-started label May 19, 2021

cmsbuild added pdmv-approved and removed pdmv-pending labels May 20, 2021

cmsbuild added geometry-approved and removed geometry-pending labels May 20, 2021

cmsbuild added reconstruction-approved and removed reconstruction-pending labels May 20, 2021

cmsbuild mentioned this pull request May 21, 2021

Run3-alca184 Add a new analyzer for studies of HCAL using isotrack and muons #33810

Merged

cmsbuild added fastsim-approved and removed fastsim-pending labels May 24, 2021

cmsbuild added alca-approved and removed alca-pending labels May 25, 2021

cmsbuild added orp-approved and removed orp-pending labels May 25, 2021

cmsbuild merged commit dfd0d1d into cms-sw:master May 25, 2021

perrotta deleted the includeTheCorrectHeader branch May 25, 2021 09:33

dan131riley mentioned this pull request May 28, 2021

Fix CaloTopologyRecord.h location #33875

Merged

cmsbuild added analysis-approved fully-signed and removed analysis-pending pending-signatures labels May 30, 2021

Include the updated header from Geometry/Records instead of the obsolete one #33784

Include the updated header from Geometry/Records instead of the obsolete one #33784

Uh oh!

Conversation

perrotta commented May 19, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR description:

PR validation:

Uh oh!

cmsbuild commented May 19, 2021

Uh oh!

cmsbuild commented May 19, 2021

Uh oh!

cmsbuild commented May 19, 2021

Uh oh!

perrotta commented May 19, 2021

Uh oh!

cmsbuild commented May 19, 2021

Build

Uh oh!

chayanit commented May 20, 2021

Uh oh!

cvuosalo commented May 20, 2021

Uh oh!

perrotta commented May 20, 2021

Uh oh!

perrotta commented May 24, 2021

Uh oh!

civanch commented May 24, 2021

Uh oh!

yuanchao commented May 25, 2021

Uh oh!

qliphy commented May 25, 2021

Uh oh!

qliphy commented May 25, 2021

Uh oh!

santocch commented May 30, 2021

Uh oh!

cmsbuild commented May 30, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

10 participants

perrotta commented May 19, 2021 •

edited

Loading